Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Add switch accounts interstitial to login flow" #10437

Merged

Conversation

ryanjduffy
Copy link
Contributor

@ryanjduffy ryanjduffy commented Mar 13, 2024

Reverts #10431

Caused a regression when launching login from the Replay browser. I didn't see it in testing because the browser was always launching to app.replay.io for login so the new local code wasn't triggered.

https://www.loom.com/share/001be5bac6af48cd957e32948feb9f83?sid=6940f0ab-9937-4058-8254-d5ff27151562

Copy link

linear bot commented Mar 13, 2024

Issue reopened: SCS-1790 Could not login the first time

Copy link

vercel bot commented Mar 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
devtools ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 13, 2024 5:57pm

Copy link

replay-io bot commented Mar 13, 2024

E2E Tests

Status In Progress ↗︎ 9 / 11
Commit e2794fa
Results
3 Failed
  • breakpoints-05: Test interaction of breakpoints with debugger statements
  • logpoints-04: should display exceptions in the console
  • repaint-05: prefers current time if pause creation failed outside of the focus window (Replay 1, Replay 2, Replay 3, Replay 4)
  • 105 Passed
  • authenticated/comments-01: Test add, edit, and delete comment functionality
  • authenticated/comments-02: Test shared comments and replies (Replay 1, Replay 2)
  • authenticated/comments-03: Comment previews
  • authenticated/logpoints-01: Shared logpoints functionality (Replay 1, Replay 2)
  • authenticated/new-test-suites/tests-01: basic tests
  • authenticated/new-test-suites/tests-02: test with no recording
  • authenticated/new-test-suites/tests-03: test ID in the URL
  • authenticated/passport-01: Time travel
  • authenticated/passport-02: Infrared inspection
  • authenticated/passport-03: Swiss army knife
  • authenticated/passport-04: Multiplayer
  • breakpoints-01: Test basic breakpoint functionality
  • breakpoints-02: Test unhandled divergence while evaluating at a breakpoint
  • breakpoints-03: Test stepping forward through breakpoints when rewound before the first one
  • breakpoints-04: catch, finally, generators, and async/await
  • breakpoints-06: Test log point in a sourcemapped file
  • breakpoints-07: rewind and seek using command bar and console messages
  • breakpoints-08: should be temporarily disabled
  • console_async: support console evaluations in async frames
  • console_dock: Should show the correct docking behavior for recordings with video
  • console_errors: Test that errors and warnings from various sources are shown in the console
  • console_eval: support console evaluations
  • console_warp-01: should support warping to console messages
  • console_warp-02: support pausing, warping, stepping and evaluating console messages
  • console-expressions-01: should cache input eager eval and terminal expressions per instance
  • cypress-01: Basic Test Suites panel functionality
  • cypress-02: Test Step timeline behavior
  • cypress-03: Test Step interactions
  • cypress-04: Test Step buttons and menu item
  • cypress-05: Test DOM node preview on user action step hover
  • deleted-recording: Show error message for deleted recording
  • elements-search: Element panel should support basic and advanced search modes
  • fe-1875 :: verify that steps go to the right point in time
  • file-search-01: should search files
  • focus_mode-01: should filter messages as regions based on the active focus mode
  • highlighter: element highlighter works everywhere
  • inspector-computed-01: Basic computed styles can be viewed
  • inspector-elements-01: Basic DOM tree node display
  • inspector-elements-02_node-picker: element picker and iframe behavior
  • inspector-elements-03: Nested node picker and selection behavior
  • inspector-elements-04: Keyboard shortcuts should select the right DOM nodes
  • inspector-elements-05_search: element picker and iframe behavior
  • inspector-rules-01: Basic CSS rules should be viewed
  • inspector-rules-03: Shorthand CSS rules should be viewed
  • jump-to-code-01: Test basic jumping functionality
  • logpoints-01: log-points appear in the correct order and allow time warping
  • logpoints-02: conditional log-points
  • logpoints-03: should display event properties in the console
  • logpoints-05: should auto-complete based on log point location
  • logpoints-06: should be temporarily disabled
  • logpoints-07: should use the correct scope in auto-complete
  • logpoints-08: should support jumping directly to a hit point via the capsule input
  • logpoints-09: should support pending edits
  • logpoints-10: too-many-points-to-find UX
  • logpoints-11: too-many-points-to-run-analysis UX
  • network-01: should filter requests by type and text
  • network-02: should show details for the selected request
  • network-03: should sync and display the current time in relation to the network requests
  • node_console_dock: Should show the correct docking behavior for recordings without video
  • node_console-01: Basic node console behavior
  • node_console-02: uncaught exceptions should show up
  • node_control_flow: catch, finally, generators, and async/await
  • node_logpoint-01: Basic node logpoints
  • node_object_preview: Showing console objects in node
  • node_quick_open_modal-01: Test basic searching functionality
  • node_spawn: Basic subprocess spawning
  • node_stepping-01: Test stepping in async frames and async call stacks
  • node_worker-01: make sure node workers don't cause crashes
  • object_preview-01: expressions in the console after time warping
  • object_preview-02: should allow objects in scope to be inspected
  • object_preview-03: Test previews when switching between frames and stepping
  • object_preview-04: Test scope mapping and switching between generated/original sources
  • object_preview-05: Should support logging objects as values
  • object_preview-06: HTML elements
  • playwright-01: Basic Test Suites panel functionality
  • playwright-02: Test Step timeline behavior
  • playwright-03: Test Step interactions
  • playwright-04: Test Step buttons and menu item
  • playwright-05: Test DOM node previews on user action step hover
  • react_devtools-01: Basic RDT behavior
  • react_devtools-02: RDT integrations (Chromium)
  • react_devtools-03: process and display multiple React versions in page
  • react_devtools-04: Component selection is maintained when seeking to a new point
  • redux_devtools: Test Redux DevTools.
  • repaint-02: repaints on hover
  • repaint-03: repaints on seek
  • repaint-04: prefers nearest (<=) paint when seeking between paints
  • resizable-panels-01: Left side Toolbar and Video should be collapsible
  • restart-session: restart debugging session
  • scopes_rerender: Test that scopes are rerendered
  • session-timeout: errors caused by session failure should bubble to the root
  • source-line-highlights: Test source line highlighting
  • sourcemap_stacktrace: Test that stacktraces are sourcemapped
  • stacking: Element highlighter selects the correct element when they overlap
  • stepping-01: Test basic step-over/back functionality
  • stepping-02: Test fixes for some simple stepping bugs
  • stepping-03: Stepping past the beginning or end of a frame should act like a step-out
  • stepping-04: Test stepping in a frame other than the top frame
  • stepping-05: Test stepping in pretty-printed code
  • stepping-06: Test stepping in async frames and async call stacks
  • stepping-07: Test quick stepping using the keyboard
  • test-suite-dashboard/test-runs-01: passed run in main branch with source
  • test-suite-dashboard/test-runs-02: failed run in temp branch without source
  • test-suite-dashboard/test-runs-03: flaky run in main branch with source
  • test-suite-dashboard/test-runs-04: test ID in the URL
  • @bvaughn
    Copy link
    Contributor

    bvaughn commented Mar 13, 2024

    I don't really understand the implications of what you're doing here so I'm just kind of rubber stamping the sign-in-flow PRs.

    @ryanjduffy ryanjduffy merged commit 2a470e0 into main Mar 13, 2024
    23 of 24 checks passed
    @ryanjduffy ryanjduffy deleted the revert-10431-ryan/scs-1790-could-not-login-the-first-time branch March 13, 2024 20:59
    ryanjduffy added a commit that referenced this pull request Mar 15, 2024
    ryanjduffy added a commit that referenced this pull request Mar 15, 2024
    * Reapply "Add switch accounts interstitial to login flow (#10431)" (#10437)
    
    This reverts commit 2a470e0.
    
    * fix handling auth requests coming from the browser
    
    * add comments
    
    * fix connection parsing; hide switch accounts on replay browser
    markerikson added a commit that referenced this pull request Mar 25, 2024
    * [RUN-3292] separately targetable steps for e2e on linux-x86_64 and macos-arm64 (#10392)
    
    Support running the e2e tests on both platforms, since we've had issues affecting only macos-arm64 in the past.
    
    In an effort to not wait on both chromium builds before starting the tests, make each step targetable using env vars.  The corresponding PR in chromium (replayio/chromium#1142) waits on a single build and then triggers this pipeline with the proper env vars.  This could also be done with multiple buildkite pipelines and/or multiple .yml files, but this seemed the best choice to keep things together.
    
    * Migrate more e2e examples to Chromium (#10396)
    
    * Add ReplayWall.sendAtPauseId() (#10399)
    
    * Replace declarative Video component with imperative code (#10393)
    
    Using a suspense cache for this was a mistake. Renders need to happen too quickly, and mixing rapid updates with suspense leads to starvation problems (renders that don't finish and update the DOM). I rewrote this to use imperative logic, which is how it was before (in a way) though I think this code is more consolidated.
    
    * Warn if hit counts can't be loaded for the outline panel (#10400)
    
    * Fixed timeline tooltip edge positioning (#10401)
    
    * Update @replayio/protocol to version 0.70.0 (#10402)
    
    * [RUN-3224] Remove `authenticated/comments-03` from blacklist (#10403)
    
    * Upgrade suspense (#10404)
    
    * Don't reset page title after a root-level error (#10406)
    
    * Don't try to load computed properties if no element is selected (#10409)
    
    * Show picker loading state until bounding rects are loaded (#10410)
    
    * Improve detection of DOM elements (#10411)
    
    * Redirect mobile clients to warning page (#10407)
    
    * Revert "Redirect mobile clients to warning page (#10407)" (#10412)
    
    This reverts commit e0b0aab because of FE-2346
    
    * Remove default roles for new members (#10370)
    
    * Remove default roles for new members
    
    * fix toggling roles instead of setting them
    
    * refactor role selection for clarity
    
    * ensure viewer role is added
    
    * Redirect mobile clients to warning page (#10413)
    
    * Redirect mobile clients to warning page (FE-2342)
    * Make test suites test-runs 2 less flaky (FE-2346)
    
    * Improve getDimensions() method error message (#10414)
    
    * Pass testScope through GraphQL Subscription Request
    
    In order to properly scope comments we need to have the testScope
    available on the websocket request. Headers on websocket requests are
    weird, you are not really supposed to use them, so we pass it through
    the same way we do the auth details.
    
    This is a noop on the backend right now, but soon it will be used.
    
    * Fix iteration in TestStepSourceLocationCache (#10417)
    
    * Don't override null fallback in InlineErrorBoundary (#10416)
    
    * Remove cloning and user-reset from e2e tests (#10418)
    
    Resolves FE-2347 and BAC-4754.
    
    This was originally added in #9497 as a way to avoid parallel tests from interfering with each other but has since proved to be difficult to maintain on the backend side. Our tests will now use unique "scopes" instead to isolate comments, points, and nags between runs.
    
    I've tested this locally by...
    - Running tests in parallel, multiple times
    - Killing tests in the middle (leaving stranded comments) and then re-running them
    
    * Improve repaint/graphics e2e test stability (#10419)
    
    I noticed some repaint e2e test failures recently. They aren't common, but when they occur, it seems like our logic of waiting for a screenshot to change is not robust enough to handle a delay in loading the initial screenshot, which can cause subsequent comparisons to fail.
    
    To reduce the likelihood of this, I've updated our test helpers to explicitly wait for the current screenshot to finish loading before returning the graphics content. I think this should improve the stability of the various repaint related tests.
    
    * Fix timing problem with elements-panel helper (#10420)
    
    * Redirect Gecko recordings to legacy.replay.io (#10421)
    
    Resolves FE-2107, FE-2132, and FE-2133
    
    - Dashboard links directly to legacy.replay.io
    - If a Gecko recording is opened on app.replay.io, show a redirect UI
    - Update e2e test recording scripts to remove "firefox" as a target
    - Temporarily disabled the last remaining Firefox e2e test (inspector-rules-02)
    
    * Disable repaint-01 e2e test (#10423)
    
    * Remove legacy (Gecko) Elements panel (#10424)
    
    - Delete legacy replay-next/components/elements code
      - Copied over the two Suspense caches and three util methods that were referenced by external code
    - Rename replay-next/components/elements-new to replay-next/components/elements
    - Update all references
    
    This is a prerequisite for making use of persistent ids in this panel.
    
    * Remove legacy (Gecko) React DevTools panel (#10425)
    
    * Rename legacy replay-next/components/elements package to replay-next/components/elements-old
    
    * Remove legacy elements panel; update references
    
    * Rename replay-next/components/elements-new to replay-next/components/elements
    
    * Udpate imperative API comment
    
    * Remove legacy, Gecko React DevTools
    
    * Harden Timeline event handlers (#10426)
    
    - Fixed a logic error in stopPlayback that caused it to null out playback state before checking if it was null (🤡)
      - I think this was the core of the bugfix
    - Move Timeline move-move and mouse-up handlers to the body element, to ensure that dragging past the timeline's boundaries updates the current time appropriately
      - This was making it difficult to scrub the playback time to the beginning of a recording, for example
    - Remove no-longer-used "dragging" state from Redux
    - Remove no-longer-used updateTime param to Redux stopPlayback action
    
    * Auto-redirect to legacy.replay.io (#10427)
    
    * Fix handling of Control key in KeyboardModifiersContext (#10432)
    
    * Allow internal users to create teams without trials (#10430)
    
    * Update doc_control_flow.html to include more precise logging (#10433)
    
    * Add switch accounts interstitial to login flow (#10431)
    
    * Add missing useEffect dependency (#10436)
    
    * Update TypeScript dep from 5.3.3 to 5.4.2 (latest) (#10428)
    
    * Update TypeScript dep from 5.3.3 to 5.4.2 (latest)
    
    * Remove `typescript` dependency from `replay-next` package
    
    * Fixed used semver range fro the `typescript` dependency
    
    * Remove redundant `typescript` dev dependency from packages
    
    * Revert "Remove redundant `typescript` dev dependency from packages"
    
    This reverts commit 8d38b36.
    
    * Unify TypeScript version used across all contained packages
    
    * Revert "Add switch accounts interstitial to login flow (#10431)" (#10437)
    
    This reverts commit 3c81069.
    
    * Delete unused replay-next test harness code (#10434)
    
    These views were useful in the initial development of replay-next components because they gave me isolated test harnesses. In more recent times, they were only exercised by the "Delta" screenshot tests, which were themselves removed in #10243.
    
    I also noticed we were running yarn test in that package as a GitHub Workflow, even though the main "Unit tests" workflow already includes these tests.
    
    * package.json and yarn.lock file auto changes
    
    * Fix tilt config to allow running deps in parallel (#10438)
    
    Allows devtools to be started sooner in the cloud dev flow. It currently
    waits until all other resources have completed before starting the devtools
    steps.
    
    * Revert "Update TypeScript dep from 5.3.3 to 5.4.2 (latest) (#10428)" (#10439)
    
    This reverts commit f5dc68b.
    
    * Update doc_rr_objects and doc_control_flow and re-enable repaint-01 (#10441)
    
    * Fix repaint-05 e2e test flakiness (#10443)
    
    * Re-enable the inspector-computed-02 e2e test (#10444)
    
    * Report session timeouts in e2e tests (#10445)
    
    * Update TypeScript dep from 5.3.3 to 5.4.2 (latest) - take 2 (#10442)
    
    * Revert "Revert "Update TypeScript dep from 5.3.3 to 5.4.2 (latest) (#10428)" (#10439)"
    
    This reverts commit 3ca214b.
    
    * Fixed the import style of `testFunction` in the Redux example
    
    * Add extra validation for a custom `playwrightScript`
    
    * Fix the inspect_element nag and the passport-02 test (#10446)
    
    * Add switch accounts interstitial to login flow (take 2) (#10440)
    
    * Reapply "Add switch accounts interstitial to login flow (#10431)" (#10437)
    
    This reverts commit 2a470e0.
    
    * fix handling auth requests coming from the browser
    
    * add comments
    
    * fix connection parsing; hide switch accounts on replay browser
    
    * Dont swallow error when recording e2e tests (#10447)
    
    * Reimplement some e2e tests (#10448)
    
    * Use different network request in authenticated/comments-03 test (#10450)
    
    * Add readme docs for playwright plugin test example (#10451)
    
    * Add more checks to passport-01 e2e test (#10452)
    
    * Improve paint/repaint behavior (#10449)
    
    Makes several small changes to graphics related code to improve interruption and better handle caching edge cases
    
    * Consider repaint graphics when finding the nearest paint (#10453)
    
    We have many layers of Suspense caching:
    1. `PaintsCache` - caches results from one-time call to `Graphics.findPaints` (array of timestamps and paint hashes)
    2. `RepaintGraphicsCache` - caches results from all calls to `DOM.repaintGraphics` (screenshots)
    3. `screenshotCache` - caches results from all calls to `Graphics.getPaintContents` (screenshots for the paint hashes cached in the `PaintsCache`)
    4. `paintHashCache` - caches screenshot data for paint hash (screenshots from `RepaintGraphicsCache` and `screenshotCache`) since the `DOM.repaintGraphics` API only sends screenshot data once per paint hash
    
    This commit merges cached paints and repaint data into a single, sorted list and updates all of the various _findClosest_ and _findMostRecent_ methods to use the merged data source. This results in less flickering when stepping between cached paint points.
    
    ### Before
    
    Graphics "flickers" when stepping because the most recent cached paint is displayed while a repaint is being fetched.
    
    https://github.com/replayio/devtools/assets/29597/10eb506a-abe4-443f-be08-e21ebdeccd87
    
    ### After
    
    No "flicker" when stepping because the nearest paint is now the recently loaded repaint.
    
    https://github.com/replayio/devtools/assets/29597/87c18646-2758-4951-a321-842c209ffa29
    
    * Add messages for the most common sources of e2e flakiness (#10454)
    
    * Update node picker e2e test logic to handle DPR > 1 (#10455)
    
    * Fix the passport-02 and inspector-elements-02 tests (#10456)
    
    * Ensure the test step details panel doesn't get stuck in loading state (#10457)
    
    * Fix selection in filtered list of tests (#10459)
    
    * Add message to addCommentHelper() (#10460)
    
    ---------
    
    Co-authored-by: Chris Toshok <ctoshok@replay.io>
    Co-authored-by: Holger Benl <hbenl@evandor.de>
    Co-authored-by: Brian Vaughn <brian.david.vaughn@gmail.com>
    Co-authored-by: Domi <Domiii@users.noreply.github.com>
    Co-authored-by: Ryan Duffy <ryan@replay.io>
    Co-authored-by: Josh Morrow <josh@jcmorrow.com>
    Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants